In applyCLIParams(), 7 storage-related parameters are written only to intermediate fields on PARAMETER, not to the PARAMETER.storage object that is actually used at runtime. The approach in applyConfigParams() is correct (writing via PARAMETER.storage.setXxx()), and applyCLIParams() should be consistent with it.

Current code in applyCLIParams:

PARAMETER.storageDbDirectory = cmd.storageDbDirectory;

Should be changed to write directly to PARAMETER.storage, consistent with applyConfigParams:

PARAMETER.storage.setDbDirectory(cmd.storageDbDirectory);

Please check the relevant parameters.

Thanks for catching this!

The existing tests did not cover the scenario where CLI storage parameters override config file values, which is why this bug went undetected after the refactor. Two test cases have been added to cover both the CLI-overrides-config and config-defaults-without-CLI scenarios.

Fixed in c4cb0e1. Please continue reviewing, thank you!

@vividctrlalt, here I noticed that seedNode was defined as a JCommander Main Parameter (without names attribute). But in applyCLIParams(), I didn't find where it's being copied to PARAMETER.seedNodes.

I might have missed it, but I couldn't find the handling in applyCLIParams(). Should we add something like:

if (!cmd.seedNodes.isEmpty()) {
    PARAMETER.seedNodes = cmd.seedNodes;
}

Apologies if I misunderstood the code flow. Please let me know if I'm missing something! Thanks for your patience.

We should also has the value check as the applyConfigParams(previously applyConfigParams) does:
PARAMETER.maxEnergyLimitForConstant = max(3_000_000L, cmd.maxEnergyLimitForConstant, true);

Thanks for the review! A couple of notes on this:

1. This is actually a pre-existing issue, not something introduced by this refactor. The original code before the refactor also directly assigns cmd.maxEnergyLimitForConstant without the max() check — so this is outside the scope of the current refactoring PR.

2. The long-term direction is to move towards a fully configuration-file-driven approach (similar to Nginx), eliminating CLI parameter overrides entirely. These CLI parameters are expected to be deprecated in the future, so investing in additional validation for them may not be worthwhile.

Thanks for the context. Agree it may have existed before, but this refactor changes the precedence model to make CLI a first-class “highest priority” layer again (fixing #6567). Once CLI overrides are honored, lack of CLI-side validation becomes more user-visible and can lead to invalid runtime state (e.g. values below the safety floor) that config-path already protects against.

Even if CLI is planned to be deprecated, we still need it to be safe and consistent while it exists. The change here is low-cost: apply the same guardrail as config (max(3_000_000L, …)), so behavior is consistent across layers and we don’t regress safety/expectations.

If you prefer not to add validation now, could we at least (1) document the constraint in the CLI help, and (2) add a test to lock the desired behavior once CLI overrides are enabled? Otherwise we risk reintroducing "config is safe but CLI can bypass safety checks" inconsistencies.

Inconsistent "explicit CLI override" detection for seedNodes: applyCLIParams() uses isAssigned() for most options, but seed nodes are applied whenever cmd.seedNodes is non-empty. With the current positional mapping, this might be fine, but it breaks the principle "only explicitly passed options override config".
If you keep seed nodes positional, please document it clearly; otherwise, prefer making it a named option and gate it via isAssigned.

Beside, in the #6567 , @halibobo1205 has mentioned this related problem.

"defalut" --> "default"

"defalut" --> "default"

"//private key" --> "// private key"

CommonParameter.reset() is marked @VisibleForTesting and documented as "Test-only", yet Args.clearParam() calls it unconditionally.

What about also adding @VisibleForTesting for clearParam()?

assigned is collected into a Map<String, ParameterDescription> but only containsKey() is used. Consider collecting into a Set<String> instead to avoid unused values and reduce the chance of Collectors.toMap throwing if keys ever collide.

Set<String> assigned = jc.getParameters().stream()
    .filter(ParameterDescription::isAssigned)
    .map(ParameterDescription::getLongestName)
    .collect(Collectors.toSet());

// Then use contains() instead of containsKey()
if (assigned.contains("--verbose")) { ... }

using "// -- Startup parameters --" instead of Unicode.

--> // --Flags (CLI + Config) --

using "// -- Network / P2P --"

using "// -- RPC / HTTP --"

using "// -- Committee parameters --"

using "// -- Netty --"

using "// -- Shielded / ZK --"

using "// -- Startup parameters --"

using "// -- Runtime parameters --"